Update QPSG logic #2066
Update QPSG logic #2066
Conversation
github-actions
bot
added
NNCF PT
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #2066 +/- ##
===========================================
+ Coverage 32.93% 36.08% +3.14%
===========================================
Files 476 476
Lines 42391 42408 +17
===========================================
+ Hits 13961 15302 +1341
+ Misses 28430 27106 -1324
|
| act_qconfig_extend_list = [] | ||
| for act_qconfig in activation_qconfig_options: | ||
| if act_qconfig.signedness_to_force is None: | ||
| for signedness_to_force_position in [True, False]: | ||
| act_qconfig_updated = deepcopy(act_qconfig) | ||
| act_qconfig_updated.signedness_to_force = signedness_to_force_position | ||
| act_qconfig_extend_list.append(act_qconfig_updated) | ||
| act_qconfig_extend_list += activation_qconfig_options | ||
| return [qconf for qconf in weight_qconfig_options if qconf in act_qconfig_extend_list] |
There was a problem hiding this comment.
Make sure that this logic covers the following case:
let weight_qconfig_options = [(*partial_configuration*, signedness_to_force=None), ]
let activation_qconfig_options = [(*same_partial_configuration*, signedness_to_force=None), ]
then the intersection should be:
intersection == [(*same_partial_configuration*, signedness_to_force=None),]
and currently it looks like your logic will produce the following instead:
intersection == [(*same_partial_configuration*, signedness_to_force=True), (*same_partial_configuration*, signedness_to_force=False)]
While we are not likely to encounter this case realistically, I think that the current behaviour in this case would be confusing.
There was a problem hiding this comment.
Currently, for the:
weight_qconfig_options = [(*partial_configuration*, signedness_to_force=None), ]andactivation_qconfig_options = [(*same_partial_configuration*, signedness_to_force=None), ]
the intersection would be:
[(*same_partial_configuration*, signedness_to_force=None), ].
This is because we save the original configuration of the activations and just add the modifications for the originals.
See 1443 line.
Added test case for that.
KodiaqQ
force-pushed
the
nm/update_qpsg_for_embeddings
branch
from
9c53dd6
to
a46cef4
Compare
github-actions
bot
added
the
NNCF OpenVINO
|
run pytorch pre-commit tests |
### Changes - Updated QPSG logic for Embedding quantization case ### Reason for changes - Proper quantization logic for QPSG in the specific case ### Related tickets - Post-merge of openvinotoolkit#2040 (CVS-113692) ### Tests - Updated `tests/common/quantization/test_quantizer_propagation_graph.py` (cherry picked from commit 23e1b87)
Changes
Reason for changes
Related tickets
Tests
tests/common/quantization/test_quantizer_propagation_graph.py